Conversation
📝 WalkthroughWalkthroughThe PR implements several enhancements and refactoring changes: migrating the SSO cache system from Symfony to WordPress transients, adding captcha token support to the checkout login flow with a pre-authentication hook, improving filter cleanup to preserve false values, adding Cloudflare troubleshooting documentation, applying minor CSS styling adjustments, and removing the symfony/cache dependency. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
inc/helpers/class-hash.php (1)
91-100:⚠️ Potential issue | 🟡 MinorAvoid
strlen()inside loop condition and use Yoda conditions.Static analysis correctly identifies two issues:
- Line 93: Calling
strlen($hash)in the loop condition recalculates the length on each iteration. While PHP may optimize this for strings, it violates the coding standard and reduces clarity.- Line 96: Should use Yoda condition:
false === $pos.🔧 Proposed fix
$number = 0; + $hash_length = strlen($hash); - for ($i = 0; $i < strlen($hash); $i++) { + for ($i = 0; $i < $hash_length; $i++) { $pos = strpos($alphabet, $hash[ $i ]); - if ($pos === false) { + if (false === $pos) { return false; } $number = $number * $base + $pos; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/helpers/class-hash.php` around lines 91 - 100, Precompute the string length and use a Yoda-style strict comparison: before the for loop that iterates over $hash (the loop using $i and $base/$number), assign $len = strlen($hash) and change the loop header to iterate up to $len, and replace the conditional if ($pos === false) with the Yoda form if (false === $pos) so the length isn't recalculated each iteration and the comparison follows the coding standard.views/dashboard-widgets/thank-you.php (1)
249-266:⚠️ Potential issue | 🟡 MinorFix indentation to match expected nesting levels.
Static analysis correctly identifies that lines within the foreach block are under-indented. The foreach statement at Line 249 should have 4 tabs (not 3), and the nested content should be indented accordingly.
The layout class additions (
wu-w-full,wu-min-w-0) are good improvements for preventing flex item overflow.🔧 Proposed indentation fix
- <?php foreach ($membership->get_sites() as $site) : ?> + <?php foreach ($membership->get_sites() as $site) : ?> - <div class="wu-bg-gray-100 wu-p-4 wu-rounded wu-mb-2 sm:wu-flex wu-items-center wu-w-full"> + <div class="wu-bg-gray-100 wu-p-4 wu-rounded wu-mb-2 sm:wu-flex wu-items-center wu-w-full">(Apply similar indentation fixes to the rest of the foreach block content)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@views/dashboard-widgets/thank-you.php` around lines 249 - 266, Adjust indentation inside the foreach loop that iterates over $membership->get_sites(): the "<?php foreach ($membership->get_sites() as $site) : ?>" line should be indented one additional level, and all nested lines (the div wrapper, img tag using $site->get_featured_image('thumbnail'), and the h5 with <?php echo esc_html(ucfirst($site->get_title())); ?>) should be shifted to match the expected nesting so each child is consistently indented under the foreach block; apply the same indentation pattern for the rest of the foreach block content to maintain consistent structure.
🧹 Nitpick comments (2)
assets/js/checkout.js (2)
1071-1098: Consider extracting captcha token collection to a reusable helper.This captcha token collection logic is duplicated from lines 912-932. Extracting it into a helper function would improve maintainability:
♻️ Optional refactor to reduce duplication
Add a helper method to the Vue instance:
getCaptchaTokens() { const tokens = {}; const recaptcha = jQuery('input[name="g-recaptcha-response"]').filter(function() { return this.value; }).first().val(); const hcaptcha = jQuery('input[name="h-captcha-response"]').filter(function() { return this.value; }).first().val(); const cap = jQuery('input[name="cap-token"]').filter(function() { return this.value; }).first().val(); if (recaptcha) tokens['g-recaptcha-response'] = recaptcha; if (hcaptcha) tokens['h-captcha-response'] = hcaptcha; if (cap) tokens['cap-token'] = cap; return tokens; },Then use it in both locations:
const login_data = { username_or_email, password: this.inline_login_password, _wpnonce: jQuery('[name="_wpnonce"]').val(), ...this.getCaptchaTokens() };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/js/checkout.js` around lines 1071 - 1098, The captcha token collection is duplicated; extract it into a reusable helper (e.g., getCaptchaTokens) on the Vue instance and replace the duplicated blocks (including the construction of inline_login_data and the earlier block at the other occurrence) by spreading the helper's return into the data object; implement getCaptchaTokens to read the three inputs ('g-recaptcha-response', 'h-captcha-response', 'cap-token'), return an object with only the present tokens, and update places that currently build inline_login_data (and the block around lines 912-932) to include ...this.getCaptchaTokens().
905-934: Captcha tokens are collected but the server-side handler doesn't pass them to the pre-auth filter.The captcha tokens (
g-recaptcha-response,h-captcha-response,cap-token) are correctly collected and sent to the server. However, examininginc/checkout/class-checkout.php(lines 1777-1788), thewu_before_inline_loginfilter only receives$usernameas context — the captcha tokens from the request are not passed to filter callbacks.Plugins hooking into
wu_before_inline_loginwill need to retrieve captcha tokens directly viawu_request('g-recaptcha-response')or similar, which works but is less explicit than passing them as filter arguments.Consider updating the filter signature to include the request data:
-$pre_auth = apply_filters('wu_before_inline_login', null, $username); +$pre_auth = apply_filters('wu_before_inline_login', null, $username, [ + 'g-recaptcha-response' => sanitize_text_field(wu_request('g-recaptcha-response')), + 'h-captcha-response' => sanitize_text_field(wu_request('h-captcha-response')), + 'cap-token' => sanitize_text_field(wu_request('cap-token')), +]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/js/checkout.js` around lines 905 - 934, The filter call for wu_before_inline_login only passes $username and should be extended to include the incoming request data (or at minimum the captcha tokens) so hooked plugins can access them; update the apply_filters('wu_before_inline_login', $username) invocation in the checkout handler (inc/checkout/class-checkout.php) to pass a second argument such as $request_data or an array of tokens (e.g., ['g-recaptcha-response'=>..., 'h-captcha-response'=>..., 'cap-token'=>...]), update the filter docblock to document the new parameter, and ensure backward compatibility by keeping $username as the first arg (so existing callbacks still receive it) and by noting that add_filter callbacks must accept the extra arg (or use func_get_args where necessary).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/apis/trait-mcp-abilities.php`:
- Around line 751-764: The foreach block that cleans up default/empty filter
values should be updated to follow WP coding standards: use spaced array keys
and Yoda comparisons. In the loop that iterates $filter_columns from
$this->get_model_filter_columns(), change any $args[$column] usages to $args[
$column ] and replace the condition checking false with a Yoda condition (false
!== $args[ $column ]). Likewise update the search check to use spaced key $args[
'search' ] and a Yoda comparison ('' === $args[ 'search' ]) in the if that
unsets search. Ensure the logic still preserves false boolean values.
In `@inc/checkout/class-checkout.php`:
- Around line 1790-1798: The pre-auth error path sends the WP_Error message
directly; update the wp_send_json_error call in the pre-auth block (where
$pre_auth is checked, and set_transient is called) to sanitize the message
before returning it — replace the raw $pre_auth->get_error_message() with a
sanitized value using wp_strip_all_tags($pre_auth->get_error_message()) so it
matches the regular authentication failure handling.
In `@inc/sso/class-sso.php`:
- Around line 917-920: The instantiation of WordPress_Simple_Cache in
class-sso.php will fail because that class is missing; fix by adding a new class
named WordPress_Simple_Cache in the inc/sso/ namespace that implements
Psr\SimpleCache\CacheInterface (implement all required methods: get, set,
delete, clear, getMultiple, setMultiple, deleteMultiple, has) using WP
transients with the provided prefix, or alternatively replace the new
WordPress_Simple_Cache('wu_sso_') call to use an existing cache helper (e.g.,
Cache_Manager or your transient wrapper) or add and import a composer package
that provides WordPress_Simple_Cache—ensure $this->cache receives a PSR-16
compatible object.
---
Outside diff comments:
In `@inc/helpers/class-hash.php`:
- Around line 91-100: Precompute the string length and use a Yoda-style strict
comparison: before the for loop that iterates over $hash (the loop using $i and
$base/$number), assign $len = strlen($hash) and change the loop header to
iterate up to $len, and replace the conditional if ($pos === false) with the
Yoda form if (false === $pos) so the length isn't recalculated each iteration
and the comparison follows the coding standard.
In `@views/dashboard-widgets/thank-you.php`:
- Around line 249-266: Adjust indentation inside the foreach loop that iterates
over $membership->get_sites(): the "<?php foreach ($membership->get_sites() as
$site) : ?>" line should be indented one additional level, and all nested lines
(the div wrapper, img tag using $site->get_featured_image('thumbnail'), and the
h5 with <?php echo esc_html(ucfirst($site->get_title())); ?>) should be shifted
to match the expected nesting so each child is consistently indented under the
foreach block; apply the same indentation pattern for the rest of the foreach
block content to maintain consistent structure.
---
Nitpick comments:
In `@assets/js/checkout.js`:
- Around line 1071-1098: The captcha token collection is duplicated; extract it
into a reusable helper (e.g., getCaptchaTokens) on the Vue instance and replace
the duplicated blocks (including the construction of inline_login_data and the
earlier block at the other occurrence) by spreading the helper's return into the
data object; implement getCaptchaTokens to read the three inputs
('g-recaptcha-response', 'h-captcha-response', 'cap-token'), return an object
with only the present tokens, and update places that currently build
inline_login_data (and the block around lines 912-932) to include
...this.getCaptchaTokens().
- Around line 905-934: The filter call for wu_before_inline_login only passes
$username and should be extended to include the incoming request data (or at
minimum the captcha tokens) so hooked plugins can access them; update the
apply_filters('wu_before_inline_login', $username) invocation in the checkout
handler (inc/checkout/class-checkout.php) to pass a second argument such as
$request_data or an array of tokens (e.g., ['g-recaptcha-response'=>...,
'h-captcha-response'=>..., 'cap-token'=>...]), update the filter docblock to
document the new parameter, and ensure backward compatibility by keeping
$username as the first arg (so existing callbacks still receive it) and by
noting that add_filter callbacks must accept the extra arg (or use func_get_args
where necessary).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62ed09a2-0e90-4197-8572-13ad803eb001
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
README.mdassets/css/legacy-signup.cssassets/css/legacy-signup.min.cssassets/js/checkout.jscomposer.jsoninc/apis/trait-mcp-abilities.phpinc/checkout/class-checkout.phpinc/helpers/class-hash.phpinc/sso/class-sso.phplang/ultimate-multisite.potreadme.txtviews/checkout/partials/inline-login-prompt.phpviews/dashboard-widgets/thank-you.php
💤 Files with no reviewable changes (1)
- composer.json
| // Remove empty filter values that AI models send as defaults | ||
| // (e.g., blog_id: 0, domain: "") which would create invalid WHERE clauses. | ||
| $filter_columns = array_keys($this->get_model_filter_columns()); | ||
|
|
||
| foreach ($filter_columns as $column) { | ||
| if (isset($args[$column]) && empty($args[$column]) && $args[$column] !== false) { | ||
| unset($args[$column]); | ||
| } | ||
| ); | ||
| } | ||
|
|
||
| // Also strip empty search strings. | ||
| if (isset($args['search']) && $args['search'] === '') { | ||
| unset($args['search']); | ||
| } |
There was a problem hiding this comment.
Fix coding style violations flagged by static analysis.
The logic correctly preserves false values for boolean filters, but there are WordPress coding standard violations:
- Array keys with variables must be surrounded by spaces:
$args[ $column ] - Use Yoda conditions:
false !== $args[ $column ]and'' === $args['search']
🔧 Proposed fix for coding style
- foreach ($filter_columns as $column) {
- if (isset($args[$column]) && empty($args[$column]) && $args[$column] !== false) {
- unset($args[$column]);
+ foreach ($filter_columns as $column) {
+ if (isset($args[ $column ]) && empty($args[ $column ]) && false !== $args[ $column ]) {
+ unset($args[ $column ]);
}
}
// Also strip empty search strings.
- if (isset($args['search']) && $args['search'] === '') {
+ if (isset($args['search']) && '' === $args['search']) {
unset($args['search']);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Remove empty filter values that AI models send as defaults | |
| // (e.g., blog_id: 0, domain: "") which would create invalid WHERE clauses. | |
| $filter_columns = array_keys($this->get_model_filter_columns()); | |
| foreach ($filter_columns as $column) { | |
| if (isset($args[$column]) && empty($args[$column]) && $args[$column] !== false) { | |
| unset($args[$column]); | |
| } | |
| ); | |
| } | |
| // Also strip empty search strings. | |
| if (isset($args['search']) && $args['search'] === '') { | |
| unset($args['search']); | |
| } | |
| // Remove empty filter values that AI models send as defaults | |
| // (e.g., blog_id: 0, domain: "") which would create invalid WHERE clauses. | |
| $filter_columns = array_keys($this->get_model_filter_columns()); | |
| foreach ($filter_columns as $column) { | |
| if (isset($args[ $column ]) && empty($args[ $column ]) && false !== $args[ $column ]) { | |
| unset($args[ $column ]); | |
| } | |
| } | |
| // Also strip empty search strings. | |
| if (isset($args['search']) && '' === $args['search']) { | |
| unset($args['search']); | |
| } |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 757-757:
Array keys must be surrounded by spaces unless they contain a string or an integer.
[failure] 756-756:
Use Yoda Condition checks, you must.
[failure] 756-756:
Array keys must be surrounded by spaces unless they contain a string or an integer.
[failure] 756-756:
Array keys must be surrounded by spaces unless they contain a string or an integer.
[failure] 756-756:
Array keys must be surrounded by spaces unless they contain a string or an integer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/apis/trait-mcp-abilities.php` around lines 751 - 764, The foreach block
that cleans up default/empty filter values should be updated to follow WP coding
standards: use spaced array keys and Yoda comparisons. In the loop that iterates
$filter_columns from $this->get_model_filter_columns(), change any
$args[$column] usages to $args[ $column ] and replace the condition checking
false with a Yoda condition (false !== $args[ $column ]). Likewise update the
search check to use spaced key $args[ 'search' ] and a Yoda comparison ('' ===
$args[ 'search' ]) in the if that unsets search. Ensure the logic still
preserves false boolean values.
| if (is_wp_error($pre_auth)) { | ||
| set_transient($transient_key, ($attempt_count ? $attempt_count + 1 : 1), 5 * MINUTE_IN_SECONDS); | ||
|
|
||
| wp_send_json_error( | ||
| [ | ||
| 'message' => $pre_auth->get_error_message(), | ||
| ] | ||
| ); | ||
| } |
There was a problem hiding this comment.
Pre-auth error messages are not sanitized, unlike regular auth failures.
The regular authentication failure path (lines 1808-1816) sanitizes the error message with wp_strip_all_tags() before sending it in the JSON response. However, the pre-auth error path sends the WP_Error message directly without sanitization.
For consistency and defense-in-depth against plugins that might include HTML in their error messages:
🛡️ Proposed fix for consistent error sanitization
if (is_wp_error($pre_auth)) {
set_transient($transient_key, ($attempt_count ? $attempt_count + 1 : 1), 5 * MINUTE_IN_SECONDS);
+ $error_message = wp_strip_all_tags($pre_auth->get_error_message());
+
+ if (empty($error_message)) {
+ $error_message = __('Login blocked. Please try again.', 'ultimate-multisite');
+ }
+
wp_send_json_error(
[
- 'message' => $pre_auth->get_error_message(),
+ 'message' => $error_message,
]
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (is_wp_error($pre_auth)) { | |
| set_transient($transient_key, ($attempt_count ? $attempt_count + 1 : 1), 5 * MINUTE_IN_SECONDS); | |
| wp_send_json_error( | |
| [ | |
| 'message' => $pre_auth->get_error_message(), | |
| ] | |
| ); | |
| } | |
| if (is_wp_error($pre_auth)) { | |
| set_transient($transient_key, ($attempt_count ? $attempt_count + 1 : 1), 5 * MINUTE_IN_SECONDS); | |
| $error_message = wp_strip_all_tags($pre_auth->get_error_message()); | |
| if (empty($error_message)) { | |
| $error_message = __('Login blocked. Please try again.', 'ultimate-multisite'); | |
| } | |
| wp_send_json_error( | |
| [ | |
| 'message' => $error_message, | |
| ] | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/checkout/class-checkout.php` around lines 1790 - 1798, The pre-auth error
path sends the WP_Error message directly; update the wp_send_json_error call in
the pre-auth block (where $pre_auth is checked, and set_transient is called) to
sanitize the message before returning it — replace the raw
$pre_auth->get_error_message() with a sanitized value using
wp_strip_all_tags($pre_auth->get_error_message()) so it matches the regular
authentication failure handling.
| if (null === $this->cache) { | ||
| // the PSR-6 cache object that you want to use | ||
| $psr6_cache = new FilesystemAdapter(); | ||
|
|
||
| $this->cache = new Psr16Cache($psr6_cache); | ||
| // Use WordPress transients-based PSR-16 cache implementation | ||
| $this->cache = new WordPress_Simple_Cache('wu_sso_'); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify WordPress_Simple_Cache class exists and implements PSR-16
# Find the WordPress_Simple_Cache class definition
echo "=== Searching for WordPress_Simple_Cache class ==="
ast-grep --pattern $'class WordPress_Simple_Cache $$$'
# Check if it implements CacheInterface
echo -e "\n=== Checking PSR-16 implementation ==="
rg -n "class WordPress_Simple_Cache" -A 5
# Check namespace/use statements in SSO class
echo -e "\n=== Checking imports in class-sso.php ==="
rg -n "use.*WordPress_Simple_Cache\|namespace.*SSO" inc/sso/class-sso.php | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 214
🏁 Script executed:
# Read the actual file at the specified lines
echo "=== Reading inc/sso/class-sso.php at lines 917-920 ==="
sed -n '915,925p' inc/sso/class-sso.php
# Search for any cache-related class definitions
echo -e "\n=== Searching for any cache-related classes ==="
rg -n "^class.*[Cc]ache" --type php
# Check if class might be in a different location
echo -e "\n=== Searching for WordPress_Simple_Cache anywhere ==="
rg "WordPress_Simple_Cache" --type php
# Check composer.json and lock file for dependencies
echo -e "\n=== Checking composer.json for cache dependencies ==="
cat composer.json | grep -A 5 -B 5 "cache\|psr"
# Look for use statements that might import this
echo -e "\n=== Checking use statements in class-sso.php ==="
head -50 inc/sso/class-sso.php | rg "^use|^namespace"Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 3578
🏁 Script executed:
# Check Cache_Manager class implementation
echo "=== Checking Cache_Manager class ==="
head -100 inc/managers/class-cache-manager.php
# Search for PSR-16 CacheInterface implementations
echo -e "\n=== Searching for CacheInterface or PSR-16 implementations ==="
rg -n "implements.*CacheInterface\|Psr.*Cache" --type php
# Check git history to see what was replaced
echo -e "\n=== Checking git diff for cache-related changes ==="
git diff HEAD~1 inc/sso/class-sso.php | head -50Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 2375
🏁 Script executed:
# Check if there are any stub/interface files for WordPress_Simple_Cache
echo "=== Searching for any cache interface files ==="
fd -t f "cache" --extension php inc/
# Look for any autoload configuration that might load this class
echo -e "\n=== Checking autoload in composer.json ==="
rg -A 20 '"autoload"' composer.json | head -40
# Check if there's a WordPress transient wrapper elsewhere
echo -e "\n=== Searching for transient-related classes ==="
rg -n "transient\|Transient" --type php -i | head -20Repository: Ultimate-Multisite/ultimate-multisite
Length of output: 889
The WordPress_Simple_Cache class does not exist and must be implemented.
The code instantiates new WordPress_Simple_Cache('wu_sso_') at line 920, but no class definition exists anywhere in the codebase. This will cause a runtime error. You must either:
- Create the
WordPress_Simple_Cacheclass in theinc/sso/directory that implements the PSR-16Psr\SimpleCache\CacheInterface, or - Use an existing cache implementation (such as the
Cache_Manageror a transient wrapper), or - Add a package to
composer.jsonthat provides this class and ensure it's properly imported.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/sso/class-sso.php` around lines 917 - 920, The instantiation of
WordPress_Simple_Cache in class-sso.php will fail because that class is missing;
fix by adding a new class named WordPress_Simple_Cache in the inc/sso/ namespace
that implements Psr\SimpleCache\CacheInterface (implement all required methods:
get, set, delete, clear, getMultiple, setMultiple, deleteMultiple, has) using WP
transients with the provided prefix, or alternatively replace the new
WordPress_Simple_Cache('wu_sso_') call to use an existing cache helper (e.g.,
Cache_Manager or your transient wrapper) or add and import a composer package
that provides WordPress_Simple_Cache—ensure $this->cache receives a PSR-16
compatible object.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
…bility Creates WordPress_Simple_Cache implementing PSR-16 CacheInterface using WordPress transients for storage. Adds use statement to SSO class to resolve missing class errors. Fixes failing unit tests: - test_cache_returns_psr16_compatible_instance - test_get_server_returns_server_instance
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
inc/sso/class-wordpress-simple-cache.php (4)
188-201: Same coding standards fixes needed fordeleteMultiple().♻️ Proposed fixes
public function deleteMultiple($keys) { - if (!is_array($keys) && !($keys instanceof \Traversable)) { + if ( ! is_array($keys) && ! ($keys instanceof \Traversable)) { throw new class('Keys must be an array or Traversable') extends \Exception implements InvalidArgumentException {}; } $success = true; foreach ($keys as $key) { - if (!$this->delete($key)) { + if ( ! $this->delete($key)) { $success = false; } } return $success; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/sso/class-wordpress-simple-cache.php` around lines 188 - 201, The deleteMultiple method should follow PSR-16 coding standards: change the signature to accept iterable $keys and return bool, and throw the proper InvalidArgumentException implementation rather than declaring an anonymous class. Update deleteMultiple to declare function deleteMultiple(iterable $keys): bool, validate $keys (throw new \Psr\SimpleCache\InvalidArgumentException with the message when invalid), iterate over $keys calling $this->delete($key) and return a boolean success flag; reference the deleteMultiple method and InvalidArgumentException type in class-wordpress-simple-cache.php when making these changes.
141-152: Address static analysis warnings for coding standards compliance.The code quality checks flag spacing issues and missing
@throwsannotations.♻️ Proposed fixes for coding standards
/** * Obtains multiple cache items. * * `@since` 2.0.11 * * `@param` iterable $keys A list of keys. * `@param` mixed $default Default value for missing keys. * `@return` iterable A list of key => value pairs. - * `@throws` InvalidArgumentException If $keys is not iterable or contains illegal values. + * `@throws` InvalidArgumentException|\Exception If $keys is not iterable or contains illegal values. */ public function getMultiple($keys, $default = null) { - if (!is_array($keys) && !($keys instanceof \Traversable)) { + if ( ! is_array($keys) && ! ($keys instanceof \Traversable)) { throw new class('Keys must be an array or Traversable') extends \Exception implements InvalidArgumentException {}; } $result = []; foreach ($keys as $key) { - $result[$key] = $this->get($key, $default); + $result[ $key ] = $this->get($key, $default); } return $result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/sso/class-wordpress-simple-cache.php` around lines 141 - 152, Add a proper docblock above getMultiple() with `@param` iterable|string[] $keys, `@param` mixed $default, `@return` array, and `@throws` \Psr\SimpleCache\InvalidArgumentException; then fix spacing to match coding standards (e.g. space around boolean operators and after commas) and replace the anonymous exception declaration with a fully-qualified InvalidArgumentException implementation (throw new class('Keys must be an array or Traversable') extends \Exception implements \Psr\SimpleCache\InvalidArgumentException {}) or better yet throw a named exception that implements \Psr\SimpleCache\InvalidArgumentException; ensure the function signature and return type remain unchanged and that the thrown exception type matches the `@throws` docblock.
164-177: Same coding standards fixes needed forsetMultiple().♻️ Proposed fixes
public function setMultiple($values, $ttl = null) { - if (!is_array($values) && !($values instanceof \Traversable)) { + if ( ! is_array($values) && ! ($values instanceof \Traversable)) { throw new class('Values must be an array or Traversable') extends \Exception implements InvalidArgumentException {}; } $success = true; foreach ($values as $key => $value) { - if (!$this->set($key, $value, $ttl)) { + if ( ! $this->set($key, $value, $ttl)) { $success = false; } } return $success; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/sso/class-wordpress-simple-cache.php` around lines 164 - 177, In setMultiple, replace the custom throw and array/Traversable check with PSR-compliant code: change the parameter to accept iterable (setMultiple(iterable $values, $ttl = null)) or at minimum use is_iterable($values) for the validation, and when throwing, instantiate an anonymous exception that extends \InvalidArgumentException and implements \Psr\SimpleCache\InvalidArgumentException (e.g. throw new class('Values must be iterable') extends \InvalidArgumentException implements \Psr\SimpleCache\InvalidArgumentException {}), leaving the rest of the loop logic intact.
115-128: Minor: Consider batching deletes for performance.The loop deletes transients one at a time. For caches with many entries, this could be slow. Also, if any delete fails, the method continues but returns
falseat the end—this is correct behavior per PSR-16.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/sso/class-wordpress-simple-cache.php` around lines 115 - 128, The current loop over $results that calls delete_transient($transient_name) for each row is slow for many entries; instead, build a batched deletion using the $wpdb API: extract the transient option names from $results (use the same $transient_name logic), construct a single DELETE query against $wpdb->options (or two queries to remove both '_transient_%' and '_transient_timeout_%' rows) using WHERE option_name IN (...) or WHERE option_name LIKE patterns, execute it and determine success by comparing affected rows to the expected count, falling back to calling delete_transient() for any names not removed; preserve the existing $success boolean behavior (set false if any expected delete did not occur) and keep the original variable names ($results, $transient_name, $success) to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@inc/sso/class-wordpress-simple-cache.php`:
- Around line 58-62: The get() and has() implementations call get_transient()
which returns false both for missing keys and for stored false values, violating
PSR-16; update get(), set(), and has() to wrap stored values in a container
(e.g., an associative array with a sentinel like ['__swc_wrapped' => true, 'v'
=> $value]) before calling set_transient and unwrap after get_transient in
get(), and detect presence in has() by checking the wrapper and sentinel rather
than raw false; keep validateKey() and the existing prefix logic but ensure
backward compatibility by treating unwrapped transients as legacy and converting
or handling them accordingly.
---
Nitpick comments:
In `@inc/sso/class-wordpress-simple-cache.php`:
- Around line 188-201: The deleteMultiple method should follow PSR-16 coding
standards: change the signature to accept iterable $keys and return bool, and
throw the proper InvalidArgumentException implementation rather than declaring
an anonymous class. Update deleteMultiple to declare function
deleteMultiple(iterable $keys): bool, validate $keys (throw new
\Psr\SimpleCache\InvalidArgumentException with the message when invalid),
iterate over $keys calling $this->delete($key) and return a boolean success
flag; reference the deleteMultiple method and InvalidArgumentException type in
class-wordpress-simple-cache.php when making these changes.
- Around line 141-152: Add a proper docblock above getMultiple() with `@param`
iterable|string[] $keys, `@param` mixed $default, `@return` array, and `@throws`
\Psr\SimpleCache\InvalidArgumentException; then fix spacing to match coding
standards (e.g. space around boolean operators and after commas) and replace the
anonymous exception declaration with a fully-qualified InvalidArgumentException
implementation (throw new class('Keys must be an array or Traversable') extends
\Exception implements \Psr\SimpleCache\InvalidArgumentException {}) or better
yet throw a named exception that implements
\Psr\SimpleCache\InvalidArgumentException; ensure the function signature and
return type remain unchanged and that the thrown exception type matches the
`@throws` docblock.
- Around line 164-177: In setMultiple, replace the custom throw and
array/Traversable check with PSR-compliant code: change the parameter to accept
iterable (setMultiple(iterable $values, $ttl = null)) or at minimum use
is_iterable($values) for the validation, and when throwing, instantiate an
anonymous exception that extends \InvalidArgumentException and implements
\Psr\SimpleCache\InvalidArgumentException (e.g. throw new class('Values must be
iterable') extends \InvalidArgumentException implements
\Psr\SimpleCache\InvalidArgumentException {}), leaving the rest of the loop
logic intact.
- Around line 115-128: The current loop over $results that calls
delete_transient($transient_name) for each row is slow for many entries;
instead, build a batched deletion using the $wpdb API: extract the transient
option names from $results (use the same $transient_name logic), construct a
single DELETE query against $wpdb->options (or two queries to remove both
'_transient_%' and '_transient_timeout_%' rows) using WHERE option_name IN (...)
or WHERE option_name LIKE patterns, execute it and determine success by
comparing affected rows to the expected count, falling back to calling
delete_transient() for any names not removed; preserve the existing $success
boolean behavior (set false if any expected delete did not occur) and keep the
original variable names ($results, $transient_name, $success) to locate the
code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da07313c-1de9-4038-b146-851e61509f6d
📒 Files selected for processing (2)
inc/sso/class-sso.phpinc/sso/class-wordpress-simple-cache.php
| public function get($key, $default = null) { | ||
| $this->validateKey($key); | ||
| $value = get_transient($this->prefix . $key); | ||
| return false !== $value ? $value : $default; | ||
| } |
There was a problem hiding this comment.
false values cannot be distinguished from cache misses.
WordPress get_transient() returns false both when a key doesn't exist and when false is the stored value. This violates PSR-16 semantics where get() must return the exact stored value. The same issue affects has() on line 214.
Consider wrapping values in a container or documenting this limitation prominently.
🛡️ Proposed fix using value wrapping
public function get($key, $default = null) {
$this->validateKey($key);
$value = get_transient($this->prefix . $key);
- return false !== $value ? $value : $default;
+ if (false === $value) {
+ return $default;
+ }
+ // Unwrap the stored value
+ return $value['v'] ?? $default;
}And in set():
public function set($key, $value, $ttl = null) {
$this->validateKey($key);
$expiration = $this->ttlToSeconds($ttl);
- return set_transient($this->prefix . $key, $value, $expiration);
+ // Wrap value to distinguish false from cache miss
+ return set_transient($this->prefix . $key, ['v' => $value], $expiration);
}🧰 Tools
🪛 GitHub Check: Code Quality Checks
[warning] 58-58:
It is recommended not to use reserved keyword "default" as function parameter name. Found: $default
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@inc/sso/class-wordpress-simple-cache.php` around lines 58 - 62, The get() and
has() implementations call get_transient() which returns false both for missing
keys and for stored false values, violating PSR-16; update get(), set(), and
has() to wrap stored values in a container (e.g., an associative array with a
sentinel like ['__swc_wrapped' => true, 'v' => $value]) before calling
set_transient and unwrap after get_transient in get(), and detect presence in
has() by checking the wrapper and sentinel rather than raw false; keep
validateKey() and the existing prefix logic but ensure backward compatibility by
treating unwrapped transients as legacy and converting or handling them
accordingly.
Summary by CodeRabbit
Documentation
New Features
Bug Fixes